Skip to content

Harden Windows PowerShell compatibility - #17

Closed
Kenny-Heitritter wants to merge 2 commits into
mainfrom
fix/windows-powershell-compatibility
Closed

Harden Windows PowerShell compatibility#17
Kenny-Heitritter wants to merge 2 commits into
mainfrom
fix/windows-powershell-compatibility

Conversation

@Kenny-Heitritter

Copy link
Copy Markdown
Member

Summary

  • make the Windows installer safe under PowerShell 5.1 by ordering helpers before use, handling native stderr probes, avoiding UTF-8 BOMs, tolerating exhausted stdin, and keeping migration variables isolated
  • normalize invalid fixed-budget thinking fields for the exact qBraid Claude model IDs that require adaptive thinking, on both Unix and Windows
  • add deterministic regression coverage and a repeatable Windows 11 VM E2E guide

Validation

  • shellcheck -S warning install.sh qbraid-code statusline.sh tests/*.sh
  • all tests/*.sh: 137 assertions passed
  • Windows PowerShell 5.1: 8 scripts parsed; PSScriptAnalyzer 0 errors; profile tests passed; 45 compatibility assertions passed
  • real Windows gateway requests passed on Claude Code 2.1.179, 2.1.186, stable 2.1.228, and 2.1.238
  • real Windows upgrade policy moved 2.1.179 to stable 2.1.228 and completed a gateway request
  • real Linux gateway request passed on Claude Code 2.1.179 without changing the installed version

Browser-backed MCP OAuth remains a separate manual check; MCP registration and compatibility fallbacks are covered.

@BeLazy167 BeLazy167 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 blocking · 3 important · 2 suggestions

The PowerShell 5.1 compatibility fixes are sound. Request handling and proxy upgrade behavior still need changes.

Comment thread install.sh
- name: "claude-opus-5"
- name: "claude-sonnet-4-6"
params:
- "thinking"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes valid adaptive thinking and output_config.effort fields from every request to these models. Current Claude clients already send that shape, so the proxy silently disables requested reasoning even though only legacy thinking.type: enabled is incompatible.

Keep adaptive requests unchanged. If compatibility handling is still needed, transform or remove only the legacy enabled shape. Do not delete output_config unconditionally.

Comment thread install.ps1
$reply = (Read-Host "$Question $hint").Trim().ToLower()
$reply = Read-Host "$Question $hint"
# Redirected or exhausted stdin makes Read-Host return null in Windows
# PowerShell 5.1. Treat it like an empty answer rather than aborting a

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redirected or exhausted input is not an affirmative answer. Converting $null to an empty string makes every default-y prompt succeed, including organization confirmation and setup changes.

Fail closed when Read-Host returns $null. Apply the default only when an interactive caller explicitly submits an empty line.

Comment thread install.ps1
$yaml += " - name: `"$gm`""
$yaml += " alias: `"$gm`""
}
$yaml += 'payload:'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template now depends on payload.filter, but the Windows installer reuses any existing cliproxyapi.exe without checking that capability. An upgrade can therefore commit this profile while an older proxy ignores or rejects the new field, leaving the original model failures in place.

Feature-detect payload filtering or enforce a supported proxy version. Replace the existing binary before committing the profile when it lacks support.

Comment thread README.md

Claude Code sends adaptive thinking, display policy, and effort for current
Opus models.
Claude Code currently sends fixed-budget thinking for qBraid's custom Opus 4.8,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says Claude Code sends fixed-budget thinking and that these requests run without extended thinking. Supported clients also send adaptive thinking with output_config.effort, and the later claim that requests pass through unchanged conflicts with this filter.

Document the actual conditional behavior after fixing the request handling. Avoid describing all requests as fixed-budget or unchanged.

Comment thread tests/install-profiles.sh
! grep -q 'key-alpha\|key-beta' "$HOME_ROOT/.claude/settings.json"; then
ok 'unsafe legacy plain-Claude credentials are removed'
else bad 'plain Claude credential cleanup'; fi
if grep -q 'name: "claude-opus-4-8"' "$BETA_DIR/proxy-template.yaml" &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These independent greps pass when the YAML is misnested or when a broader filter also strips thinking from untargeted models.

Parse the generated YAML or run it through the proxy. Assert one targeted model changes while Haiku and GPT requests keep their thinking fields.

@BeLazy167

BeLazy167 commented Aug 21, 2026

Copy link
Copy Markdown
Member

[GPT-5.6-sol] RESPONDING ON BEHALF OF BeLazy167

Closing as superseded by #18. That PR includes the PowerShell 5.1 ordering, native-process handling, encoding, migration, and native Windows lifecycle coverage from this change.

The remaining adaptive-thinking filter should not merge. It removes valid adaptive thinking and output_config fields and assumes proxy filter support without capability detection. qBraid/qbraid-api#1025 owns the server-side compatibility fix.

@BeLazy167 BeLazy167 closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants